-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-14410] [SQL] : SessionCatalog needs to check database/table/function existence #12183
Conversation
Pulling functionality from apache spark
pull latest from apache spark
Pulling functionality from apache spark
Pulling functionality from apache spark
pull request from apache/master
pull latest from apache spark
pull latest from apache spark
Test FAILed. |
@rekhajoshm Thank you for working on this! I think there are other methods in SessionCatalog that handle an existing table/db/function that do not check the existence of the table/db/function (e.g. dropFunction). Can you go through methods provided by SessionCatalog and make changes accordingly? |
@yhuai - will do, one question is., as database calls had ignoreIfExists/ignoreIfNotExists flag, now with adding existence check this boolean loses meaning.Do we still pass that flag? If not, more changes across in clients.Please let me know.thanks! |
externalCatalog.createDatabase(dbDefinition, ignoreIfExists) | ||
if (!databaseExists(dbDefinition.name)) { | ||
externalCatalog.createDatabase(dbDefinition, ignoreIfExists) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For createDatabase, we should throw an exception if db already exists and ignoreIfExists is false, right?
@rekhajoshm I think there is some misunderstanding on what the issue is. The JIRA is saying all the create/drop methods in |
@rekhajoshm I've opened #12198 to illustrate what I mean. I don't think the changes in this patch are correct so I would recommend that we close this PR. |
ok thanks @andrewor14 |
What changes were proposed in this pull request?
1.Check for existence of Function in catalog, instead of try-catch
2. Check for existence of database before create/alter/drop database, though ignoreIfExists/ignoreIfNotExists become futile
3.Rename getFunction to getMetadataOfFunction
How was this patch tested?
TestSuite